Skip to content

Conversation

@maarquitos14
Copy link
Contributor

Breaks were lost in 5fd4877, restoring them. When the breaks are lost, literals are enconded/decoded twice, which is wrong.

** This is done in this repo because this piece of code is not present in Khronos repo. **

When the breaks are lost, literals are enconded/decoded twice, which is wrong.

Signed-off-by: Marcos Maronas <marcos.maronas@intel.com>
@maarquitos14 maarquitos14 requested a review from a team as a code owner June 27, 2024 10:23
@maarquitos14 maarquitos14 changed the title [spirv-tools] Restoring breaks incorrectly dropped [spirv] Restoring breaks incorrectly dropped Jun 27, 2024
Copy link
Contributor

@vmaksimo vmaksimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

@maarquitos14
Copy link
Contributor Author

@intel/llvm-gatekeepers I think this is ready to merge.

@sommerlukas
Copy link
Contributor

@maarquitos14 Can you please check your Github account settings to include your actual mail address in the commit for PRs:

This commit will be authored by maarquitos14@users.noreply.github.com

We can only merge after this is fixed.

@maarquitos14
Copy link
Contributor Author

@maarquitos14 Can you please check your Github account settings to include your actual mail address in the commit for PRs:

This commit will be authored by maarquitos14@users.noreply.github.com

We can only merge after this is fixed.

This is very weird, I have had several PRs merged in the last year and a half without doing this. Did anything change in Github?

@sommerlukas
Copy link
Contributor

Not that I'm aware of, but maybe this was just missed on previous PRs.

The note to gatekeepers to look out for this is relatively new (as in a few months old).

@sommerlukas sommerlukas merged commit b609233 into intel:sycl Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants